Report more errors to the background script and test runner #686
Conversation
noisysocks
left a comment
noisysocks
left a comment
There was a problem hiding this comment.
Good idea. I made very similar changes to playwright/runner.ts locally when I was debugging issues.
Code looks good to me aside from question around whether it’s a good idea to have AutoConsent leak information about the messaging channel.
Could you please provide testing instructions?
| domActions: DomActions; | ||
| filtersEngine: FiltersEngine; | ||
| protected sendContentMessage: MessageSender; | ||
| sendContentMessage: MessageSender; |
There was a problem hiding this comment.
Maybe _runRulesSequentially in AutoConsentCMP should throw an error that AutoConsent catches? We're mixing concerns somewhat by having AutoConsentCMP know about the messaging channel.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| msg: 'Rule step failed', | ||
| details: `${JSON.stringify(rule)}`, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Bug: Failed detection rules incorrectly reported as errors
The _runRulesSequentially method now sends an autoconsentError message whenever a non-optional rule step returns false. This is problematic because this method is called by detectCmp() and detectPopup() during CMP detection. When checking if a CMP is present, most CMPs' detection rules are expected to fail (since typically only one CMP is on a page). The new code treats these normal detection failures as errors, flooding the message channel with false "errors" for every CMP that isn't present and every failed detection step. This makes test output less helpful, not more, as real errors become buried in noise.
Please tell me if this was useful or not with a 👍 or 👎.
2 participants
A minor tweak to make the test output more helpful
Note
Send
autoconsentErrormessages on rule failures and detect/popup errors; enable CI to log these and pass detailed log config; update tests to stub messaging.autoconsentErrorwhen a rule step fails in_runRulesSequentially(lib/cmps/base.ts).detectCmpanddetectPopup/waitForPopup, sendautoconsentErrorwith CMP name and error message (lib/web.ts).autoconsentErrormessages; pass explicitlogsconfig ininitResp(playwright/runner.ts).sendContentMessagein mocks to accommodate new error reporting (tests-wtr/rules/logical-rules.test.ts).Written by Cursor Bugbot for commit ab47a3a. This will update automatically on new commits. Configure here.